Skip to content

KAFKA-18699: Cleanup Metadata Module #20346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Aug 22, 2025
Merged

KAFKA-18699: Cleanup Metadata Module #20346

merged 4 commits into from
Aug 22, 2025

Conversation

sjhajharia
Copy link
Contributor

@sjhajharia sjhajharia commented Aug 12, 2025 •
edited by github-actions bot
Loading

https://issues.apache.org/jira/browse/KAFKA-18699

This PR aims at cleaning up the metadata module further by getting rid
of some extra code which can be replaced by record

Reviewers: Ken Huang [email protected], Ming-Yen Chung
[email protected], Chia-Ping Tsai [email protected]

@github-actions github-actions bot added triage PRs from the community kraft labels Aug 12, 2025
mingyen066
mingyen066 reviewed Aug 14, 2025
View reviewed changes
Copy link
Collaborator

@mingyen066 mingyen066 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup for migrating suitable classes to records has a corresponding JIRA ticket: KAFKA-18696.

For metadata cleanup, you can take my ticket: KAFKA-18699.

metadata/src/main/java/org/apache/kafka/controller/BrokerControlStates.java Outdated
return other.current == current && other.next == next;
}

record BrokerControlStates(BrokerControlState current, BrokerControlState next) {
@Override
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not remove toString as well? As far as I know, the only difference is that the parentheses become square brackets.

@github-actions github-actions bot removed the triage PRs from the community label Aug 14, 2025
@sjhajharia sjhajharia changed the title MINOR: Cleanup Metadata Module KAFKA-18699: Cleanup Metadata Module Aug 14, 2025
@sjhajharia
Copy link
Contributor Author

Thanks @mingyen066
I have assigned the JIRA to myself and as per your suggestion got rid of some of the toString() methods as well. I did not touch the ones which use Stringify in them.
Re-requesting a review.

@sjhajharia sjhajharia requested a review from mingyen066 August 14, 2025 05:46
m1a2st
m1a2st reviewed Aug 14, 2025
View reviewed changes
Copy link
Collaborator

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sjhajharia for this patch, left some comments

@sjhajharia
Copy link
Contributor Author

Thanks @m1a2st for the review. I have addressed the comments. Pls re-review. Thanks!

@sjhajharia sjhajharia requested a review from m1a2st August 15, 2025 07:40
@sjhajharia
Copy link
Contributor Author

Adding @chia7712 as he has been kindly helping review a whole lot of PRs in this domain.
TIA!

chia7712
chia7712 reviewed Aug 21, 2025
View reviewed changes
Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjhajharia thanks for this cleanup!

@sjhajharia
Copy link
Contributor Author

Thanks @chia7712 for the initial review and the feedback. I have addressed the same.
Pls re-review when possible.

@sjhajharia sjhajharia requested a review from chia7712 August 22, 2025 14:05
@chia7712 chia7712 merged commit de92b20 into apache:trunk Aug 22, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chia7712 chia7712 chia7712 approved these changes

@mingyen066 mingyen066 Awaiting requested review from mingyen066

@m1a2st m1a2st Awaiting requested review from m1a2st

Assignees
No one assigned
Labels
ci-approved kraft
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sjhajharia @chia7712 @mingyen066 @m1a2st @frankvicky